[LIVY-621]add dynamic service discovery for thrift server#193
Conversation
Codecov Report
@@ Coverage Diff @@
## master #193 +/- ##
===========================================
+ Coverage 68.54% 68.6% +0.05%
Complexity 906 906
===========================================
Files 100 100
Lines 5674 5688 +14
Branches 854 854
===========================================
+ Hits 3889 3902 +13
- Misses 1228 1229 +1
Partials 557 557
Continue to review full report at Codecov.
|
|
Hi @mgaido91 , could you please help to take a look at this PR. |
|
@jerryshao @vanzin I am pinging you because I am not sure about this PR. I mean, AFAIK Livy has no mechanism currently to create a "cluster" of Livy servers which interact among them, neither it has any rebalancing at all. I may be wrong, so that's why I am pinging you. If I am not wrong on the above, I'd be against this PR which basically creates a cluster of thriftserver without having a cluster of Livy servers. I think we should first implement a feature like this for Livy server itself, and then eventually leverage it on the thriftserver part. |
|
Yes @mgaido91 , currently Livy doesn't have "cluster" itself, so the discovery mechanism seems more like a way to know LivyServer URL from ZK, #189 also has a similar proposal, and I left the similar comment in JIRA-616. Without Livy "cluster" support, a such discovery mechanism may not super useful/necessary. |
|
Hi @jerryshao , #189 is for Rest API, and this one is for thrift API. This PR is to make livy thrift server compatibility with existing hiveserver2 client, jdbc or beeline, so user can move quickly from hiveserver to livy. |
|
Hi @mgaido91 @jerryshao, thanks a lot for your comments! Some considerations :
We have dozens of hiveserver2 in product deployment, it will help our users to move smoothly from hive to spark with service discovery, because it is almost impossible to let users to know all of these server instances. |
|
the point here is: from the project perspective we should first achieve the same level of HA and robustness for the Livy server part. Then, we can also have it for the thriftserver. It'd be very weird that HA is available only for the thrift part and not for the REST API for instance. So I think that in order to have this, we should first wait for #189. Once that is over, we can port the same functionality to the thrift part, eventually leveraging the solution already present there. |
|
Sure, that is a good point. |
|
Livy user in my company is asking for Rest API Load Balance functionality these days. So I go throught #189 and #212 and then I realize service discovery for Rest is quite different with Thrift. The main difference is thrift is a long connection while rest is not. When thrift connect is break, the session is shutdown but rest session is not. And another difference is thrift's client are exsiting hive client while rest have many different clients. |
|
I agree they are different and they may also lead to separate solutions, but I'd prefer to work on a full proposal which covers both cases. You may want to create a design doc and start a discussion maybe, proposing the solution you consider more appropriate. Once the design doc is approved I am happy to go ahead with this. |
|
Hi @mgaido91 @jerryshao , I create a JIRA https://issues.apache.org/jira/browse/LIVY-698 to propose a cluster solution, could you please take a look. |
|
This pull request has been automatically marked as stale because it has had no activity for at least 3 months. If you are still working on this change or plan to move it forward, please leave a comment or push a new commit so we know to keep it open. Otherwise, this PR will be closed automatically in about one month. Thank you for your contribution to Apache Livy! |
What changes were proposed in this pull request?
Add config and implementation to allow publish livy thrift server to zookeeper. Configuration information are consistent with hiveserver2 for convenience. Publish information is the same as hiveserver2 has, so beeline can discover thrift server.
https://issues.apache.org/jira/browse/LIVY-621
How was this patch tested?
add unit test. And have tested manually.